Plotting Improvements - Series color scale and color picker options#1903
Plotting Improvements - Series color scale and color picker options#1903
Conversation
- ChartColorInputs for single color geomOptions and series specific color and shape value map - ChartConfig measuresOptions to store per series mapping object - Hide and show color options based on selected chart type and measures
…ith min width at 330px
… (like boolean true/false)
labkey-alan
left a comment
labkey-alan
left a comment
There was a problem hiding this comment.
Overall this looks good, and is working for me locally (though I'm not fully done with manual testing). I did leave some feedback that should probably get addressed (though I know some of it may need to be deferred due to time constraints), but nothing major. I will attach additional comments as I complete manual testing.
packages/components/src/internal/components/chart/ChartColorInputs.tsx
Outdated
Show resolved
Hide resolved
| @@ -1,22 +1,98 @@ | |||
| import React, { FC, memo, useCallback, useState } from 'react'; | |||
| import React, { FC, memo, useCallback, useRef, useState } from 'react'; | |||
There was a problem hiding this comment.
ESLint is complaining about this file, be sure to run eslint --fix before merging.
There was a problem hiding this comment.
yup, that is on my pre merge checklist (but I always wait until after CR so that it is easier to see the actual dev changes for the reviewer)
packages/components/src/internal/components/forms/input/ColorPickerInput.tsx
Outdated
Show resolved
Hide resolved
packages/components/src/internal/components/forms/input/ColorPickerInput.tsx
Outdated
Show resolved
Hide resolved
| > | ||
| {text ? text : value ? <ColorIcon cls="color-picker__chip-small" asSquare value={value} /> : 'None'} | ||
| {text ? text : value ? <ColorIcon cls="color-picker__chip-small" asSquare value={value_} /> : noValueText} | ||
| <i className={classNames('fa', { 'fa-caret-up': showPicker, 'fa-caret-down': !showPicker })} /> |
There was a problem hiding this comment.
If we have the time I think we should try to style this to match the select component. It looks pretty jarring to have the two components next to each other.
| export interface MeasureOption { | ||
| color?: string; | ||
| shape?: string; | ||
| // lineType?: string; |
There was a problem hiding this comment.
It's probably safe to go ahead and uncomment lineType, since that's coming in a follow-on PR shortly after this one.
packages/components/src/internal/components/forms/input/ColorPickerInput.test.tsx
Outdated
Show resolved
Hide resolved
Manual Testing Log:Color picker renders awkwardly on smaller screen sizesWhen looking at a screen that is half the width of my 1440p monitor the color picker for the "color" option of a line chart renders awkwardly. The remove icon renders below the picker. The remove icon for Shape renders next to the picker (as I would expect). There seems to be enough space to render everything on one line though. Mismatched styles for ColorPickerInput and SelectInputThe ColorPickerInput and SelectInput render differently in a few ways that are pretty jarring when they're rendered next to each other:
|
…and styling update
2 participants
Rationale
The next round of plotting improvements focus on the color options. For most chart types, this PR adds color pickers for things like line color, fill color, and point color. For line charts that have a series measure selected, this adds to the chart builder modal options to be able to select a specific series value and set the color and shape for that series. These settings are all then stored with the chart config object and used on render.
Related Pull Requests
Changes